Fix case-insensitive charset detection for ISO-2022-JP - #13641
neconohitomi wants to merge 1 commit into
Conversation
| $normalizedCharset = $this->normalizeCharset($charset); | ||
| try { | ||
| if (in_array($normalizedCharset, $this->mbEncodings(), true)) { | ||
| if (in_array(strtoupper($charset), array_map('strtoupper', $this->mbEncodings()), true)) { |
There was a problem hiding this comment.
Please move array_map(strtoupper) into the mbEncodings function.
| $normalizedCharset = $this->normalizeCharset($charset); | ||
| try { | ||
| if (in_array($normalizedCharset, $this->mbEncodings(), true)) { | ||
| if (in_array(strtoupper($charset), array_map('strtoupper', $this->mbEncodings()), true)) { |
There was a problem hiding this comment.
strtoupper($charset) should be done in normalizedCharset. At best by altering the map to also use uppercase and flip lowerCharset to upperCharset and return that version as fallback.
|
Good finding, Thanks for your pr 👍 |
|
The commit message needs a DCO signoff and the Assisted-By trailer. It's often enough to point your Agent https://github.com/nextcloud/mail/blob/main/AGENTS.md#commit-message-format and https://github.com/nextcloud/mail/pull/13641/checks?check_run_id=101341532634 to resolve it. |
Thanks for the review and for pointing out that the current commit is missing the required DCO I used an AI agent (ChatGPT) while preparing this PR, and this makes me wonder about the following situation. I would appreciate your advice. According to What would be the preferred way to fix the DCO/ I can prepare the required fixup commit if that is the preferred approach. |
Force push |
Assisted-by: ChatGPT:gpt-5.6 Signed-off-by: neconohitomi <registry@tsumu.net>
39e1f5f to
139e94a
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
This branch has not been deployed
2 participants
🤖 AI (if applicable)
Summary
Fixes ISO-2022-JP email conversion when the charset is provided in lowercase.
Problem
Some emails declare their charset as
ISO-2022-JP, but Horde can return the charset as lowercase (iso-2022-jp).Converterpreviously checked the charset againstmb_list_encodings()using a case-sensitive comparison. As a result,iso-2022-jpwas not recognized as an mbstring encoding and the conversion fell back toiconv().On the affected Alpine/libiconv environment,
iconv()fails to convert these ISO-2022-JP messages, resulting in garbled email content.Fix
Make the charset comparison case-insensitive when checking the available mbstring encodings.
The existing charset normalization and conversion logic are otherwise unchanged.
Tests
Added a regression test covering lowercase
iso-2022-jp.Tested with:
ConverterTest: 15 tests, 30 assertions — OKFixes #13472